Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Dead-time correction algorithm and UI elements for settings #95

Merged
merged 16 commits into from
Aug 9, 2024
Merged

Dead-time correction algorithm and UI elements for settings #95

merged 16 commits into from
Aug 9, 2024

Conversation

backmari
Copy link
Collaborator

@backmari backmari commented Jul 3, 2024 •
edited
Loading

Short description of the changes:

This branch combines back-end changes by @mdoucet in the branch: https://github.com/neutrons/reflectivity_ui/tree/dead_time
and adds UI changes by @searscr and @backmari.

Long description of the changes:

  • new Mantid-style algorithm SingleReadoutDeadTimeCorrection
  • logic to apply dead-time correction when a file is loaded
  • UI elements to configure dead-time correction settings (applied globally for all runs)
  • logic to reload files upon changes in the dead-time correction settings

TODO: add unit testing of the new algorithm SingleReadoutDeadTimeCorrection

Check list for the pull request

  • I have read the [CONTRIBUTING]
  • I have read the [CODE_OF_CONDUCT]
  • I have added tests for my changes
  • I have updated the documentation accordingly

Check list for the reviewer

  • I have read the [CONTRIBUTING]
  • I have verified the proposed changes
  • best software practices
    • all internal functions have an underbar, as is python standard
    • clearly named variables (better to be verbose in variable names)
    • code comments explaining the intent of code blocks
  • All the tests are passing
  • The documentation is up to date
  • code comments added when explaining intent

Manual test for the reviewer

  1. Open the file
    REF_M_42112+42113_Specular_SA_test_dead-time_datarepo.dat.txt
    and edit the 4 file paths to point to the location of your data repo folder. (Can remove the .txt filename suffix added to fool GitHub.)
  2. Load the reduced file in QuickNXS.
  3. Verify that the "Dead Time Correction" options are available in the UI and test that checking/unchecking the checkbox enables/disables the "Settings" button.
  4. The files in the reduction table and direct beam table need to be reloaded when the dead-time settings are changed, and the user is asked to confirm before reloading. Verify that all other configuration stays the same when dead-time settings are changed.
  5. Verify that the reflectivity plot is updated when dead-time settings are changed.

image

References

Story 5023: [QUICKNXS] Add dead time options to UI

@codecov-commenter
Copy link

codecov-commenter commented Jul 3, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 52.38095% with 120 lines in your changes missing coverage. Please review.

Project coverage is 44.96%. Comparing base (3a7d763) to head (9b869a7).

Files Patch % Lines
..._ui/interfaces/data_handling/DeadTimeCorrection.py 22.41% 45 Missing ⚠️
...ectivity_ui/interfaces/data_handling/instrument.py 36.36% 28 Missing ⚠️
...ivity_ui/interfaces/event_handlers/main_handler.py 10.00% 27 Missing ⚠️
reflectivity_ui/interfaces/data_manager.py 57.14% 9 Missing ⚠️
reflectivity_ui/ui/deadtime_settings.py 83.78% 6 Missing ⚠️
reflectivity_ui/ui/deadtime_entry.py 88.88% 4 Missing ⚠️
reflectivity_ui/interfaces/main_window.py 88.88% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             next      #95      +/-   ##
==========================================
+ Coverage   35.43%   44.96%   +9.52%     
==========================================
  Files          30       33       +3     
  Lines        6067     6310     +243     
==========================================
+ Hits         2150     2837     +687     
+ Misses       3917     3473     -444

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@backmari backmari changed the title Dead time settings Dead-time correction settings Jul 3, 2024
@backmari backmari changed the title Dead-time correction settings Dead-time correction algorithm and UI elements for settings Jul 3, 2024
@backmari
Copy link
Collaborator Author

backmari commented Jul 3, 2024

Comments from Mat:

  • need to recalculate the reflectivity when files are reloaded
  • does the cache need to cleared?
  • reload only the files in the tables and not all in the cache?
  • keep the settings when reloading the files
  • default to dead-time settings off

@backmari backmari force-pushed the dead_time_settings branch from 70360ec to 2453c64 Compare August 2, 2024 15:09
jmborr
jmborr requested changes Aug 5, 2024
View reviewed changes
Copy link
Member

@jmborr jmborr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add instructions for a "Manual Test"?

reflectivity_ui/interfaces/data_handling/DeadTimeCorrection.py Show resolved Hide resolved
reflectivity_ui/interfaces/configuration.py Show resolved Hide resolved
reflectivity_ui/interfaces/data_handling/instrument.py Show resolved Hide resolved
reflectivity_ui/interfaces/data_handling/instrument.py Show resolved Hide resolved
reflectivity_ui/interfaces/data_handling/instrument.py Show resolved Hide resolved
@jmborr
Copy link
Member

jmborr commented Aug 5, 2024

@backmari codecov says the patch coverage is only 49%. You introduced some new functions. Did you write unit tests for them?

mdoucet and others added 16 commits August 9, 2024 07:41
@mdoucet @backmari
add dead time
3b78869
@mdoucet @backmari
add dead time algo
6bd105e
@pre-commit-ci @backmari
[pre-commit.ci] auto fixes from pre-commit.com hooks
0b25153 
for more information, see https://pre-commit.ci
@mdoucet @backmari
Update DeadTimeCorrection.py
9024b40
@pre-commit-ci @backmari
[pre-commit.ci] auto fixes from pre-commit.com hooks
b665b75 
for more information, see https://pre-commit.ci
@searscr @backmari
add UI elements for dead time settings
5da5650
@backmari
Store deadtime settings in global configuration and att unit tests
5d5badb
@backmari
reload files upon change in dead time settings
6162ae8
@backmari
test that changing deadtime settings triggers reloading files
e2b05b3
@backmari
Address review comments
914caa7 
- dead-time correction should be turned off by default
- reload only files in tables (not all cached files)
- keep the run settings when reloading files
- recalculate reflectivity when reloading files
@backmari
fix typo
9f9c7e5
@backmari
fix bug where reduction table configuration would change when updatin…
729d259 
…g dead-time settings and reloading files
@backmari
add used documentation
bdd2e9e
@backmari
add unit tests for dead-time correction
c36c121
@backmari
add docstrings and test of function reload_all_files
2076606
@backmari
add release note and unit test for mantid_algorithm_exec
9b869a7
@backmari backmari force-pushed the dead_time_settings branch from dcce379 to 9b869a7 Compare August 9, 2024 12:58
@backmari backmari merged commit b432f41 into next Aug 9, 2024
3 checks passed
@backmari backmari deleted the dead_time_settings branch August 9, 2024 15:59
@backmari backmari mentioned this pull request Dec 5, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmborr jmborr jmborr approved these changes

Assignees

@backmari backmari

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@backmari @codecov-commenter @jmborr @mdoucet @searscr